-
Notifications
You must be signed in to change notification settings - Fork 641
dataconnect: EnumValue and EnumValueSerializer added #7153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
dataconnect: EnumValue and EnumValueSerializer added #7153
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces EnumValue
and EnumValueSerializer
to handle enums that may have values not known at compile time. The review suggests refactoring the EnumValue
sealed class implementations to use Kotlin's data class
feature to reduce boilerplate.
firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/EnumValue.kt
Outdated
Show resolved
Hide resolved
firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/EnumValue.kt
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
…i changes (go/dataconnect:sdk:enums)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces enum value support components for the firebase-dataconnect library, adding EnumValue
types and serialization infrastructure to handle both known and unknown enum values gracefully.
- Adds
EnumValue
sealed interface withKnown
andUnknown
implementations for forward-compatible enum handling - Implements
EnumValueSerializer
for kotlinx.serialization support - Updates integration tests to demonstrate enum value serialization with subset mappings
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/EnumValue.kt | Core EnumValue sealed interface and implementations |
firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/serializers/EnumValueSerializer.kt | Kotlinx serialization support for EnumValue |
firebase-dataconnect/src/test/kotlin/com/google/firebase/dataconnect/EnumValueKnownUnitTest.kt | Unit tests for EnumValue.Known implementation |
firebase-dataconnect/src/test/kotlin/com/google/firebase/dataconnect/EnumValueUnknownUnitTest.kt | Unit tests for EnumValue.Unknown implementation |
firebase-dataconnect/src/test/kotlin/com/google/firebase/dataconnect/serializers/EnumValueSerializerUnitTest.kt | Unit tests for EnumValueSerializer |
firebase-dataconnect/src/androidTest/kotlin/com/google/firebase/dataconnect/EnumIntegrationTest.kt | Integration tests demonstrating enum value usage |
firebase-dataconnect/api.txt | API surface updates for new public classes |
firebase-dataconnect/gradle.properties | Version bump to 17.1.0 |
firebase-dataconnect/CHANGELOG.md | Documentation of new features |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...se-dataconnect/src/androidTest/kotlin/com/google/firebase/dataconnect/EnumIntegrationTest.kt
Outdated
Show resolved
Hide resolved
...se-dataconnect/src/androidTest/kotlin/com/google/firebase/dataconnect/EnumIntegrationTest.kt
Outdated
Show resolved
Hide resolved
...se-dataconnect/src/androidTest/kotlin/com/google/firebase/dataconnect/EnumIntegrationTest.kt
Show resolved
Hide resolved
/gemini review |
Summary of ChangesThis pull request integrates robust support for user-defined enums within the firebase-dataconnect library. It provides a flexible EnumValue type that can represent both expected and unexpected enum strings, along with a dedicated serializer. This foundational change prepares the library for future updates where these types will be directly consumed, enhancing API stability and developer experience. Highlights
Changelog
Activity
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces EnumValue
and EnumValueSerializer
to support user-defined enums with forward compatibility in firebase-dataconnect
. The implementation is robust and well-tested with both unit and integration tests. My review includes a couple of suggestions to enhance code clarity and conciseness, primarily by leveraging Kotlin's data class
feature and improving the structure of test data classes for better maintainability.
...se-dataconnect/src/androidTest/kotlin/com/google/firebase/dataconnect/EnumIntegrationTest.kt
Show resolved
Hide resolved
firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/EnumValue.kt
Show resolved
Hide resolved
The change #7153 was not part of the 17.0.1 release, so it should be part of the unreleased section. Also, since it introduces API changes, it requires a minor version bump.
The change #7153 was not part of the 17.0.1 release, so it should be part of the unreleased section. Also, since it introduces API changes, it requires a minor version bump.
The change #7153 was not part of the 17.0.1 release, so it should be part of the unreleased section. Also, since it introduces API changes, it requires a minor version bump.
This pull request introduces components for supporting user-defined enums in the
firebase-dataconnect
library. It defines theEnumValue
type capable of representing both recognized and unrecognized enum values, along with a dedicated serializer forkotlinx.serialization
. These types duplicate those generated by the Data Connect code generator; however, in the future the code generator will cease to generate them and, instead, use the types added in this PR.Highlights
EnumValue
, a sealed interface designed to represent enum values. It has two concrete implementations:EnumValue.Known
for recognized enum members andEnumValue.Unknown
for string values that do not map to a known enum member. This design allows for graceful handling of forward compatibility, such as when new enum values are added to an API.EnumValueSerializer
, akotlinx.serialization.KSerializer
implementation forEnumValue
. This serializer handles converting enum strings toEnumValue
objects (mapping toKnown
orUnknown
as appropriate) and serializingEnumValue
objects back to their string representation.Google employees can see go/dataconnect:sdk:enums for full details.